Skip to content

London | Jan-26 | Abdul Moiz | Sprint 2 | Form Controls #954

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
A-Moiz wants to merge 2 commits into
base: main
Choose a base branch
from
Open

London | Jan-26 | Abdul Moiz | Sprint 2 | Form Controls #954

A-Moiz wants to merge 2 commits into from

Conversation

@A-Moiz
Copy link

@A-Moiz A-Moiz commented Jan 15, 2026

Learners, PR Template

Self checklist

  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • My changes meet the requirements of the task
  • I have tested my changes
  • My changes follow the style guide

Changelist

I created a form and ticked off the checklist in README.md file

Questions

N/A

@netlify
Copy link

netlify bot commented Jan 15, 2026 •
edited
Loading

✅ Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit 04c0e4b
🔍 Latest deploy log https://app.netlify.com/projects/cyf-onboarding-module/deploys/6972510f5b86bc00080692ac
😎 Deploy Preview https://deploy-preview-954--cyf-onboarding-module.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		 2 paths audited
Performance: 100 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 86 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify project configuration.

@A-Moiz A-Moiz added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jan 15, 2026
@A-Moiz A-Moiz mentioned this pull request Jan 17, 2026
Open
@cjyuan cjyuan added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jan 21, 2026
cjyuan
cjyuan reviewed Jan 21, 2026
View reviewed changes
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1

According to https://validator.w3.org/, there are errors in your code. Can you fix them?

Suggestion

After you have made all the changes, share your code to an AI tool and ask it to review your code in terms of semantic, accessibility, typo, and consistency.
It can often help us catch errors, improve consistency, and reinforce best practices.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jan 21, 2026
@A-Moiz
Copy link
Author

A-Moiz commented Jan 22, 2026

Hi @cjyuan , I made a new commit resolving all the errors and changes you listed. Please have another look at it and let me know if anymore changes are needed. Thanks.

@cjyuan
Copy link
Contributor

cjyuan commented Jan 23, 2026

  • Changes addressed the comments. Good job.

  • The wording and letter case of the labels could be made more consistent.

Note: In CYF courses, in addition to responding to reviewer's comments, trainees should also add "Needs review" label when their PR is ready to be re-reviewed. Without the "Needs review" label, the reviewer won't know if a PR is ready to be reviewed.

@A-Moiz
Copy link
Author

A-Moiz commented Jan 23, 2026

Hi, @cjyuan thanks for that I'll keep that in mind for future PRs. I can see that the Completed label hasn't been added yet to this PR. Are anymore changes required?

@cjyuan
Copy link
Contributor

cjyuan commented Jan 23, 2026

Without the proper label, I would not know if this PR is ready to be re-reviewed.

@A-Moiz A-Moiz added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Submit:PR This work is submitted by creating a PR against CodeYourFuture's fork of this repo. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Reviewed Volunteer to add when completing a review with trainee action still to take. Submit:PR This work is submitted by creating a PR against CodeYourFuture's fork of this repo. labels Jan 23, 2026
@cjyuan cjyuan added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jan 23, 2026
@A-Moiz
Copy link
Author

A-Moiz commented Jan 23, 2026 •
edited
Loading

Hi @cjyuan , I added the Needs review label back but I was getting an error from GitHub validator stating that I also need to add a Submit label and after adding one I was getting the same error. It currently only has 1 label (Needs review) and 1 error from GitHub validator. Please let me know if this is okay for now or if I would need to make any changes to this PR. Thanks.

EDIT: I can see that you just added the Complete label and the checks have passed. Apologies for the inconvenience and thanks for reviewing this PR.

@cjyuan
Copy link
Contributor

cjyuan commented Jan 23, 2026

Don't worry about the error. It was caused by an issue some trainee mistakenly created on CYF's account. The error is gone now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cjyuan cjyuan cjyuan left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

Complete Volunteer to add when work is complete and all review comments have been addressed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@A-Moiz @cjyuan